Support JRA55do in datm#407
Conversation
billsacks
left a comment
billsacks
left a comment
There was a problem hiding this comment.
These changes basically look good to me. I appreciate your maintaining the error check with these changes.
I have one request: can you change the name of the new jra55do variable? Currently the logic that triggers this to be true will only be true for the JRA55 stream, but it seems like it could easily be true in other cases in the future, and it would be confusing to have this name in that case. Can you name it something like stream_separates_rain_snow?
|
@mvertens - assigning you to review for approval from the NorESM side. @alperaltuntas - assigning you to review since this impacts ocean model forcing options. For both of you, feel free to reassign or remove yourself as reviewer if you want. |
|
Thanks @billsacks - i've made that change |
billsacks
left a comment
billsacks
left a comment
There was a problem hiding this comment.
Better, thanks... but can you please correct the spelling? -- "seperates" should be "separates". (Sorry for the nitpick... spelling errors in variable names bug me more than they probably should.)
mvertens
left a comment
mvertens
left a comment
There was a problem hiding this comment.
Looks good. Thanks for the clean implementation.
|
Shall I add jra55do to https://escomp.github.io/CDEPS/versions/master/html/datm.html#supported-data-modes ? We don't use CIME, so it may not entirely make sense with the rest of the page |
billsacks
left a comment
billsacks
left a comment
There was a problem hiding this comment.
Looks good now - thank you!
Actually, that would be great - thanks. My sense is that that page is out of date in other respects, but whatever you can add there relatively easily would be great! I'll wait until you give me the go-ahead - for that and anything else you may want to do - before merging. |
|
Thanks for adding to the docs! I'll merge shortly. |
4 participants
Description of changes
This adds support for JRA55do within
datm- namely, rain and snow can be seperate forcing variables (currently assumed to be the same forcing variable)Specific notes
Contributors other than yourself, if any:
Are there dependencies on other component PRs (if so list): None
Are changes expected to change answers (bfb, different to roundoff, more substantial): bfb
Any User Interface Changes (namelist or namelist defaults changes):
No namelist changes
JRA55doto existing options fordatamodeindatm_inFaxa_prsn(snow) andFaxa_prrn(rain) in datm.streams.xml filesTesting performed (e.g. aux_cdeps, CESM prealpha, etc):
Hashes used for testing:
0b2d3bd